Skip to content

Migrate pallet-im-online to TransactionExtension API#11235

Open
RomarQ wants to merge 5 commits intoparitytech:masterfrom
RomarQ:rq/im-online-migration
Open

Migrate pallet-im-online to TransactionExtension API#11235
RomarQ wants to merge 5 commits intoparitytech:masterfrom
RomarQ:rq/im-online-migration

Conversation

@RomarQ
Copy link
Copy Markdown
Contributor

@RomarQ RomarQ commented Mar 3, 2026

Summary

Part of #2415 (follow-up to #10150)

Migrate pallet-im-online from the deprecated ValidateUnsigned trait to #[pallet::authorize], following the pattern established in #10716.

Changes

  • Replace ValidateUnsigned impl with #[pallet::authorize] on the heartbeat call.
  • Change Config bound from CreateBare<Call<Self>> to CreateAuthorizedTransaction<Call<Self>>.
  • ensure_noneensure_authorized, create_barecreate_authorized_transaction.
  • Add tests covering all authorize validation paths and dispatch with RawOrigin::Authorized.

Migration

The Config trait now requires CreateAuthorizedTransaction<Call<Self>> instead of CreateBare<Call<Self>>. Runtimes with a blanket impl<C> CreateAuthorizedTransaction<C> for Runtime (like kitchensink) need no changes.

@RomarQ RomarQ requested a review from a team as a code owner March 3, 2026 09:36
Comment on lines +399 to +404
#[pallet::authorize(|_source: TransactionSource,
heartbeat: &Heartbeat<BlockNumberFor<T>>,
signature: &<T::AuthorityId as RuntimeAppPublic>::Signature,
| -> TransactionValidityWithRefund {
if Pallet::<T>::is_online(heartbeat.authority_index) {
// we already received a heartbeat for this authority
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be my personal preference to have a code block this big somewhere outside of the pallet macros, and just calling it by function implemented somewhere else.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, moved the closure outside in 1ed6f34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants